exec-builtin: flag non-call references in preview mode - #28051
exec-builtin: flag non-call references in preview mode#28051CAOShurong wants to merge 3 commits into
Conversation
|
ntBre
left a comment
ntBre
left a comment
There was a problem hiding this comment.
Thanks, this looks good overall. I think we can consolidate the stable and preview checks, though.
| // Exec | ||
| // Calls to `exec` are reported by [`exec_used`]; here we only flag | ||
| // non-call references (see `suspicious_function_reference`) to avoid | ||
| // duplicate diagnostics. | ||
| ["" | "builtins", "exec"] if arguments.is_none() => { |
There was a problem hiding this comment.
Should we just remove the check in exec_used? It just seems confusing to split the check if both could be handled here, which I think is the case.
There was a problem hiding this comment.
If we make this change, which I still think we should, we need to use func.range() here to preserve the range from exec_used:
ruff/crates/ruff_linter/src/rules/flake8_bandit/rules/exec_used.rs
Lines 36 to 41 in d580d7e
There was a problem hiding this comment.
Let's update this list to include S102.
| /// S102, S301, S302, S303, S304, S305, S306, S307, S308, S310, S311, S312, S313, S314, S315, S316, S317, S318, S319, S320, S321, S323 |
ntBre
added
rule
`exec-builtin` (S102) only flagged direct calls to the builtin `exec`, while `suspicious-eval-usage` (S307) also flags references such as `map(eval, ...)` or `foo = eval` in preview mode. Extend the preview-mode reference detection in `suspicious_function` to cover `exec` so both rules behave consistently. Calls remain reported once via the existing call-site check; references are preview-gated behind the same `suspicious_function_reference` machinery used by S307. Fixes astral-sh#28011
c0c7351 to
a221927
Compare
2 participants
Summary
exec-builtin(S102) currently flags only direct calls to the builtinexec, while the parallel rulesuspicious-eval-usage(S307) also flags non-call references likemap(eval, ...)orfoo = evalin preview mode (behavior added in #15522/#15523). This makes the two rules inconsistent:This extends the existing preview-mode reference detection (
suspicious_function_reference) to coverexec, so both rules treat references consistently. The reference diagnostics are gated behind the same preview flag as S307's, so stable-mode output is unchanged.Calls are still reported exactly once via the existing call-site check (
exec_used); the new arm only matches non-call references to avoid duplicate diagnostics.Fixes #28011
Test Plan
flake8_bandit/S102.py:list(map(exec, ["hi"]))andfoo = exec(both flagged in preview), plus a shadowed localexec(not flagged).preview_rulestest case forExecBuiltin/S102.py; snapshot shows exactly the two added reference diagnostics vs. stable mode.cargo test -p ruff_linter --release --libgreen (2797 passed);cargo fmt --checkclean.